-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Merge the two apt RUN instructions in Dockerfile.updater-core #13684
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: jeffwidman <[email protected]>
| apt purge software-properties-common apt-transport-https -y | ||
| apt-get autoremove -y | ||
| rm -rf /var/lib/apt/lists/* | ||
| EOT |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot what does this EOT syntax do? Are these commands still executed as part of the RUN command or are they copied to a file instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The RUN <<EOT ... EOT syntax is the Dockerfile heredoc feature introduced in BuildKit (Docker 1.20+). All commands between <<EOT and EOT are executed as part of the RUN instruction—nothing is copied to a file.
This syntax allows writing multi-line shell scripts more naturally without && \ line continuations. It also enables embedding nested heredocs (like the <<EOF ... EOF for creating the apt sources file) which wouldn't work with the traditional RUN syntax.
See the Docker documentation: https://docs.docker.com/build/building/best-practices/#heredocs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoa, cool!
|
@yeikel what's your take on this? We have an issue where millions of Dependabot jobs are resulting in each What do you think of this PR? |
| mercurial \ | ||
| # needed to sign commits | ||
| gnupg2 \ | ||
| # for git repo access with ssh key credentials |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These comments are important context, can we ask copilot to bring them back? @jeffwidman
That, or bring them back manually. Whatever works best
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot can you please leave the code comments inline and not delete them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yeikel good spot. I was only looking at the actual dockerfile RUN structure change and completely missed that!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like I'll have to manually fixup this commit... that's okay, at least we're a lot closer. Let's see if CI is happy
Thanks for the context, at Dependabot's scale that's quite the challenge (an exciting one though!) Is the issue only about layer calls or also due to the size of the images?(ie: pulls?) I am asking because some images are quite large (ie: Maven) and makes me curious if we can find opportunities there as well
I think that the approach makes sense. I am not concerned about readability because EOT makes it very readable. As part of this change, we may be able to consolidate further. See #13685
When we run However, the registry configuration created in this layer is stored in |
What are you trying to accomplish?
Combines the Azure mirror configuration RUN (lines 17-59) and apt-get installation RUN (lines 61-109) into a single layer. This is the base image, so reducing layers will mean millions fewer calls from the Docker image registry.
Fixes #12341
Anything you want to highlight for special attention from reviewers?
RUN set -e; ... && ...chaining toRUN <<EOTheredoc syntax to cleanly embed the nested<<EOFfor the sources file/etc/apt/sources.list.d/ubuntu.sources) persists in the final layer—child images still inherit it for their own apt-get callsHow will you know you've accomplished your goal?
docker buildx build --check -f Dockerfile.updater-core .passes syntax validationChecklist
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.